Skip to content

Mostly copy in .gitattributes from master#84

Merged
JeroenDeDauw merged 2 commits intoDataValues:0.4.xfrom
reedy:04xgitattributes
Sep 24, 2019
Merged

Mostly copy in .gitattributes from master#84
JeroenDeDauw merged 2 commits intoDataValues:0.4.xfrom
reedy:04xgitattributes

Conversation

@reedy
Copy link
Contributor

@reedy reedy commented Sep 19, 2019

No description provided.

@JeroenDeDauw
Copy link
Member

Master has it wrong. tests/ cannot be excluded since it is (unfortunately) not fully package private. https://github.com/DataValues/Common/blob/master/composer.json#L47

@JeroenDeDauw
Copy link
Member

Master fix: #85

@reedy
Copy link
Contributor Author

reedy commented Sep 19, 2019

Master has it wrong. tests/ cannot be excluded since it is (unfortunately) not fully package private. https://github.com/DataValues/Common/blob/master/composer.json#L47

What do you mean?

Tests shouldn't be needed when installing via composer...

@JeroenDeDauw
Copy link
Member

There is a test base class used by other packages. Bad design from 2012 says hello. https://github.com/DataValues/Common/blob/master/tests/ValueParsers/ValueParserTestBase.php

@JeroenDeDauw
Copy link
Member

Been removing code using shit like this for years. This base class still has some users. Help with killing those is welcome

JeroenDeDauw added a commit that referenced this pull request Sep 20, 2019
Allows for #84

Wikibase Repo has 3 usages of StringValueParserTest:
https://github.com/wikimedia/mediawiki-extensions-Wikibase/search?q=StringValueParserTest

These can easily be fixed by using a copy of the class
(or better yet: refactoring away from this inheritance abuse pattern).
@reedy reedy changed the title Copy in .gitattributes from master Mostly copy in .gitattributes from master Sep 24, 2019
@JeroenDeDauw JeroenDeDauw merged commit 727bba5 into DataValues:0.4.x Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants